Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use super for errors to propagate cause #58

Merged
merged 2 commits into from
Feb 14, 2023
Merged

Conversation

FZambia
Copy link
Member

@FZambia FZambia commented Feb 14, 2023

Alternative solution for #55

@PhilipDukhov
Copy link
Contributor

Actually, I don't know Java that deeply, mostly from how it interacts with kotlin=)

After discovering super looks much better than initCause 👍

@PhilipDukhov
Copy link
Contributor

PhilipDukhov commented Feb 14, 2023

Also, if you decide to start from scratch, feel free to ask me for some suggestions and help with the API.

I'd do it in Kotlin, as it requires much less boilerplate - like you can use default parameters and no need to duplicate methods for different parameter variants.

But even if you decide to stick with Java for your own reasons, there're some stuff I can help with.

e.g. in Java world builder pattern is often used, so you can build things like options inline, like

new Client(
    endpoint, 
    Options.Builder()
		.setToken("")
		.build(), 
    listener
);

An other thing I'd change is updating ResultCallback to be safer - right now you can pass both exception and success result, which is ambiguous

@FZambia
Copy link
Member Author

FZambia commented Feb 14, 2023

Thanks, I will keep this in mind 👍 It was my first and the only Java project, so some not idiomatic and naive things may be here and there. Luckily some things may be gradually improved. I suppose builder may be introduced in backwards compatible way. Harder with ResultCallback I think... Our Swift SDK utilizes Result type.

That's what I was able to find for Java:

public class Result<T> {
    private T result;
    private Throwable error;

    private Result(T result, Throwable error) {
        this.result = result;
        this.error = error;
    }

    public static <T> Result<T> success(T result) {
        return new Result<>(result, null);
    }

    public static <T> Result<T> error(Throwable error) {
        return new Result<>(null, error);
    }

    public T getResult() {
        return result;
    }

    public Throwable getError() {
        return error;
    }

    public boolean isSuccess() {
        return error == null;
    }

    public boolean isError() {
        return error != null;
    }
}

You mean sth like this right?

Coming back to pr, so let's proceed with this PR then instead of #54.

@PhilipDukhov
Copy link
Contributor

Yeah, you can even use kotlin.Result - it should work in Java, not sure how good is its API from Java side, but for kotlin lib users it'll be nice =)

@FZambia
Copy link
Member Author

FZambia commented Feb 14, 2023

I guess it's a good thing, but adding types from Kotlin goes beyond my current understanding :) I'd prefer keeping things simple here, to avoid problems with maintaining. I just opened a separate issue for keeping Result type in mind.

@FZambia FZambia changed the title Use super for errors Use super for errors to propagate cause Feb 14, 2023
@FZambia FZambia merged commit 857d55d into master Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants